-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inventory Rewrite - new simple-inventory plugin #5164
Conversation
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
to inventory core plugin Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
to inventory plugin Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
…ory plugin Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
when ordering Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
and related server changes Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Remaining code comments have been addressed |
@kieckhafer - can you take another look at this in hopes that we can try and get it merged and over to SDI today? |
Signed-off-by: Eric Dobbertin <[email protected]>
@kkuzemka will re-test first thing this morning. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting this error inside migration 18. Ran it a few times and confirmed it happens every time. Migrations stop at 18 because of this.
Error while migrating TypeError: doc.layout is not iterable
at imports/plugins/core/versions/server/migrations/18_use_react_for_header_and_footer_layout.js:42:5
at Array.forEach (<anonymous>)
at Object.up (imports/plugins/core/versions/server/migrations/18_use_react_for_header_and_footer_layout.js:28:5)
at migrate (packages/percolate_migrations.js:233:25)
at Object.Migrations._migrateTo (packages/percolate_migrations.js:253:7)
at Object.Migrations.migrateTo (packages/percolate_migrations.js:167:10)
at appEvents.on (imports/plugins/core/versions/server/startup.js:44:9)
at Promise.asyncApply (imports/node-app/core/util/appEvents.js:24:11)
at /home/node/.meteor/packages/promise/.0.11.2.1gwulfb.vjny++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/meteor-promise/fiber_pool.js:43:40
When I ran from an existing install, which was already up to version 62, migration 63 ran OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another migration error... If i manually set Migrations to 19, to allow them to continue to run, migration 22 has an issue:
Error while migrating TypeError: Cannot read property 'registry' of undefined
at Object.up (imports/plugins/core/versions/server/migrations/22_register_verify_account.js:9:5)
at migrate (packages/percolate_migrations.js:233:25)
at Object.Migrations._migrateTo (packages/percolate_migrations.js:253:7)
at Object.Migrations.migrateTo (packages/percolate_migrations.js:167:10)
at appEvents.on (imports/plugins/core/versions/server/startup.js:44:9)
at Promise.asyncApply (imports/node-app/core/util/appEvents.js:24:11)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside form those two migrations, after I get them all to run (by manually pushing it past the broken ones in the database), I get missing index errors on a lot of indexes. Here is one example:
MongoError: index not found with name [c2_billing.$.paymentMethod.items.$.productId]
at /home/node/.meteor/packages/npm-mongo/.3.1.1.v3rpzk.m5kk8++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/mongodb-core/lib/connection/pool.js:581:63
at authenticateStragglers (/home/node/.meteor/packages/npm-mongo/.3.1.1.v3rpzk.m5kk8++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/mongodb-core/lib/connection/pool.js:504:16)
at Connection.messageHandler (/home/node/.meteor/packages/npm-mongo/.3.1.1.v3rpzk.m5kk8++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/mongodb-core/lib/connection/pool.js:540:5)
at emitMessageHandler (/home/node/.meteor/packages/npm-mongo/.3.1.1.v3rpzk.m5kk8++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/mongodb-core/lib/connection/connection.js:310:10)
at Socket.<anonymous> (/home/node/.meteor/packages/npm-mongo/.3.1.1.v3rpzk.m5kk8++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/mongodb-core/lib/connection/connection.js:453:17)
at emitOne (events.js:116:13)
at Socket.emit (events.js:211:7)
at addChunk (_stream_readable.js:263:12)
at readableAddChunk (_stream_readable.js:250:11)
at Socket.Readable.push (_stream_readable.js:208:10)
at TCP.onread (net.js:597:20)
I get over 70 errors for missing indexes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get a white screen, with the following error when trying to go to localhost:3000
:
TypeError: Cannot read property 'sort' of undefined
at ReactionLayout (router.js:436)
at Function.Router.initPackageRoutes (router.js:548)
at initBrowserRouter (browserRouter.js:167)
at startup.js:40
at Computation._compute (tracker.js:308)
at Computation._recompute (tracker.js:324)
at Object.Tracker._runFlush (tracker.js:495)
at onGlobalMessage (meteor.js?hash=33066830ab46d87e2b249d2780805545e40ce9ba:504)
That line is
const sortedLayout = shop.layout.sort((prev, next) => prev.priority - next.priority);
so considering the migrations that were broken had to do with layout, my guess is this is related.
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After you place an order for a variant, if you look at the Inventory card for that variant in the operator UI, you should see the ordered quantity listed as reserved.
If you approve the fulfillment group, the reserved quantity should decrease and the in stock quantity should decrease.
When I approve a fulfillment group, the reserved quantity decreased, but the in-stock quantity stayed the same.
If you cancel an item before approving, the reserved quantity should decrease and the in stock quantity should stay the same.
This is happening, but it's happening whether I click return to stock or not... should this be the case? Should it only happen if I return to stock?
If you cancel an item after approving and choose to "return to stock", the reserved quantity should stay the same and the in stock quantity should increase.
I cannot test this, as mentioned in the first point, the quantity did not decrease when I approved.
If you cancel an item after approving and choose NOT to "return to stock", the reserved quantity should stay the same and the in stock quantity should stay the same.
I cannot test this, as mentioned in the first point, the quantity did not decrease when I approved.
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
@aldeed All above mentioned issues related to the Orders UI seem to be fixed. Still seeing the issues we discussed with startup and packages loading. |
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
Signed-off-by: Eric Dobbertin <[email protected]>
I fixed the issue with Package registry being blank on first run with blank DB. Migration 3 was clearing it so I commented out migration 3. 😅 I doubt anyone is upgrading from that far back, and it isn’t needed for a fresh install, so it seems like a safe way to fix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 With the startup changes, I think this is ready to go.
Resolves #4215
Resolves #4931
Resolves #4932
Resolves #5107
Resolves #5123
Impact: breaking
Type: feature|performance
Changes
Overview
context.collections
. It was no longer used. It is not dropped, though, in case anyone is using it.Catalog
collection, on thevariants
andoptions
objects. Now all of these fields are removed from the database except three top-level product bools that must be in the collection data for sorting and filtering purposes. This doesn't affect GraphQL, though, because those fields are still set at read time.AUTO_PUBLISH_INVENTORY_FIELDS
environment variable no longer affects anything.buildOrderItem
, part of the process of placing an order, is updated to query inventory using the new APICore Inventory Plugin
There was already a core
inventory
plugin, but much of the inventory code was still scattered in other plugins. Now all inventory code is in the coreinventory
plugin, and plugins do not import or reference inventory code outside of the standard ways (context).Here's what the core
inventory
plugin now does:product.isSoldOut
,product.isLowQuantity
, andproduct.isBackorder
booleans on aCatalogItem
record when it is published. Then keeps these values correct whenever inventory information changes for a variant. We must store these fields in the DB rather than adding them at read time because we allow sorting and/or filtering by them.CatalogProductVariant
andCartItem
at read time by registering transform functions that thecatalog
plugin calls.context.queries.inventoryForProductConfigurations
, and a convenience function for a single configuration,context.queries.inventoryForProductConfiguration
, as the standard way for other plugins to retrieve inventory info.simple-inventory
plugin, for the actual data.isSoldOut
,isLowQuantity
, andisBackorder
as supported boolean filters for thecatalogItems
query.Simple Inventory Plugin
As mentioned above, the core
inventory
plugin requires one or more inventory plugins to do the actual inventory tracking for product configurations (variants).In general, an inventory plugin needs to do the following:
inventoryForProductConfigurations
type function that returns current inventory in stock and other related data for a list of product configurations.A new included
simple-inventory
plugin does all of the above in a manner similar to the way inventory worked before this PR. The UI has been improved a bit but is still in the same place. One major difference is that the actual inventory data is now stored in a newSimpleInventory
collection rather than onProducts
, which makes it much easier to read and write. A migration populatesSimpleInventory
collection from data currently onProducts
, so the change should be transparent.One UI improvement is regarding the "reserved" quantity for a variant. The UI now shows in stock, reserved, and available numbers, and there is a button that allows you to force the "reserved" number to update if it becomes incorrect. This plugin considers inventory to be reserved when it's part of an order that has been placed but is not yet approved.
How to sync inventory quantities from an external system
Option 1 If your external inventory system can be queried in real time, the simplest integration is to remove the
simple-inventory
plugin and add your own custom inventory plugin that registers aninventoryForProductConfigurations
type function that returns current inventory in stock and other related data for a list of product configurations by calling through to your inventory management system.Option 2 If your external inventory system produces periodic exports but cannot be queried in real time, then you may want to keep the included
simple-inventory
plugin and create a script that reads the exported files and updates theSimpleInventory
collection as necessary. If you do this in a custom plugin, then it's best to callcontext.mutations.updateSimpleInventory
withcontext.isInternalCall
set totrue
, and let that take care of properly updating the collection. If you can't do it in a custom plugin, then your external script should be written to look similar to theupdateSimpleInventory
mutation function in thesimple-inventory
plugin.Breaking changes
The GraphQL API is mostly unchanged. Inventory fields for
CatalogProductVariant
andCartItem
are as they were. But internally, everything is different. So custom plugins that manage or use inventory are likely to need a rewrite.The example web storefront will break unless you have the changes in reactioncommerce/example-storefront#527.
Testing
This change touches a lot of areas, and although most have decent test coverage, we should carefully test everything that might be affected.
inventoryReserved
number to an incorrect number in theSimpleInventory
collection and verify that it is fixed when clicking the "Recalculate reserved quantity" buttoncatalogItems
GraphQL query works.